pkg/krknai/analysisengine/engine.go (1)

254-260: ⚠️ Potential issue | 🔴 Critical

Sanitize rendered markdown before casting to template.HTML.

Line 259 marks renderer output as trusted HTML. With LLM/log-derived markdown, this enables HTML/script injection in the generated report.

🔒 Suggested hardening

 import (
 	"bytes"
 	"context"
 	"embed"
 	"fmt"
 	"html/template"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"time"
 
 	"github.com/gomarkdown/markdown"
 	mdhtml "github.com/gomarkdown/markdown/html"
 	"github.com/gomarkdown/markdown/parser"
+	"github.com/microcosm-cc/bluemonday"
 	"github.com/openshift/osde2e/internal/analysisengine"
 	"github.com/openshift/osde2e/internal/llm"
 	"github.com/openshift/osde2e/internal/llm/tools"
 	"github.com/openshift/osde2e/internal/prompts"
@@
-	body := markdown.ToHTML([]byte(content), p, renderer)
+	unsafeBody := markdown.ToHTML([]byte(content), p, renderer)
+	safeBody := bluemonday.UGCPolicy().SanitizeBytes(unsafeBody)
@@
-	if err := tmpl.Execute(&buf, struct{ Body template.HTML }{Body: template.HTML(body)}); err != nil {
+	if err := tmpl.Execute(&buf, struct{ Body template.HTML }{Body: template.HTML(string(safeBody))}); err != nil {
 		return "", fmt.Errorf("failed to execute HTML template: %w", err)
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/krknai/analysisengine/engine.go` around lines 254 - 260, The rendered
markdown byte slice stored in variable body (from markdown.ToHTML called
alongside parser.NewWithExtensions and mdhtml.NewRenderer) is currently cast
directly to template.HTML before tmpl.Execute, allowing untrusted HTML/script
injection; fix this by sanitizing the renderer output before casting: add a
trusted sanitizer (e.g., bluemonday.StrictPolicy or a policy that allows safe
markdown tags) and run it on body (or string(body)), then cast the sanitized
result to template.HTML when building the struct passed to tmpl.Execute; ensure
imports are added and replace the direct template.HTML(body) usage with
template.HTML(sanitizedBody).

internal/prompts/prompts_test.go (1)

10-18: Add a focused test for RegisterTemplates overwrite behavior.

Good assertion for default availability. Since this PR adds template overlay semantics, add one test that registers a same-ID template and verifies the default gets replaced.

🧪 Proposed test addition

 import (
 	"testing"
+	"testing/fstest"

 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )

+func TestRegisterTemplates_OverridesExistingTemplate(t *testing.T) {
+	store, err := NewPromptStore(DefaultTemplates())
+	require.NoError(t, err)
+
+	overrideFS := fstest.MapFS{
+		"default.yaml": &fstest.MapFile{
+			Data: []byte("system_prompt: override-system\nuser_prompt: override-user\n"),
+		},
+	}
+
+	require.NoError(t, store.RegisterTemplates(overrideFS))
+
+	tmpl, err := store.GetTemplate("default")
+	require.NoError(t, err)
+	assert.Equal(t, "override-system", tmpl.SystemPrompt)
+	assert.Equal(t, "override-user", tmpl.UserPrompt)
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/prompts/prompts_test.go` around lines 10 - 18, Add a focused unit
test that verifies RegisterTemplates overwrites existing templates: create a
PromptStore via NewPromptStore(DefaultTemplates()), then call
store.RegisterTemplates(...) with a template object using the same ID (e.g.,
"default") but different content, retrieve it via store.GetTemplate("default")
and assert the template's content now matches the newly registered one (and not
the original). Reference NewPromptStore, DefaultTemplates, RegisterTemplates,
and GetTemplate in the test to locate the code paths to exercise.

pkg/krknai/analysisengine/prompts/krknai.yaml (1)

17-17: Reduce hallucination risk for expected status codes.

You require expected status codes in output; make the artifact-read instruction explicit for that field so the model doesn’t infer missing values.

✍️ Suggested prompt tweak

-  ## Test Configuration (GA params; list all enabled chaos scenarios; health check targets with name, endpoint URL, and expected status code)
+  ## Test Configuration (GA params; list all enabled chaos scenarios; health check targets with name, endpoint URL, and expected status code; fetch from artifacts when not present in prompt variables)

@@
-  Use read_file on relevant artifacts. Generate the full markdown report per system prompt structure.
+  Use read_file on relevant artifacts (especially krkn-ai.yaml for health-check expected status codes). Generate the full markdown report per system prompt structure.

Also applies to: 63-63

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/krknai/analysisengine/prompts/krknai.yaml` at line 17, The prompt's "##
Test Configuration" section currently asks for expected status codes but doesn't
force the model to read the artifact; update the krknai.yaml prompt to add an
explicit artifact-read requirement for the expected status code field (e.g.,
require "expected status code" or "expected_status_code" be populated only from
the provided artifact and never guessed), and mirror this change for the other
occurrence around line 63 so the model must extract that value from the artifact
rather than infer or fabricate it.

pkg/krknai/aggregator/aggregator.go (1)

103-107: Defensively copy ClusterInfo to avoid pointer aliasing side effects.

WithClusterInfo and Collect currently share the same pointer instance. A caller mutating info later can unintentionally change previously collected output.

♻️ Proposed defensive-copy update

 func (a *KrknAIAggregator) WithClusterInfo(info *ClusterInfo) *KrknAIAggregator {
-	a.clusterInfo = info
+	if info == nil {
+		a.clusterInfo = nil
+		return a
+	}
+	copied := *info
+	a.clusterInfo = &copied
 	return a
 }
 
 func (a *KrknAIAggregator) Collect(ctx context.Context, resultsDir string) (*KrknAIData, error) {
@@
-	data := &KrknAIData{
-		ClusterInfo: a.clusterInfo,
-	}
+	data := &KrknAIData{}
+	if a.clusterInfo != nil {
+		copied := *a.clusterInfo
+		data.ClusterInfo = &copied
+	}

Also applies to: 117-119

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/krknai/aggregator/aggregator.go` around lines 103 - 107, WithClusterInfo
currently stores the provided *ClusterInfo pointer directly causing aliasing;
change it to store a defensive copy instead (e.g., implement or call a
ClusterInfo.Clone/Copy and assign that to KrknAIAggregator.clusterInfo) so later
mutations by the caller don't change stored data; also ensure any use in Collect
reads from that copied instance (or makes another copy before including in
output) to avoid sharing the original pointer.

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting